-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[release/8.0-staging] GT_STORE_BLK - do not call memset for blocks containg gc pointers on heap #96514
[release/8.0-staging] GT_STORE_BLK - do not call memset for blocks containg gc pointers on heap #96514
Conversation
Co-authored-by: Qiao Pengcheng <qiaopengcheng@loongson.cn>
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsBackport of #95530 to release/8.0-staging /cc @EgorBo Customer ImpactTestingRiskIMPORTANT: If this backport is for a servicing release, please verify that:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
You should probably run outerloop and jitstress jobs to ensure the 8.0 sources haven't diverged in important ways from main.
/azp run runtime-coreclr outerloop, runtime-coreclr jitstress |
Azure Pipelines successfully started running 2 pipeline(s). |
@EgorBo Friendly reminder that Tuesday January 16th 4pm is the Code Complete deadline for the February Release. If all requirements are met, please merge your PR before that date and time to ensure this fix gets included in that Release. |
@jeffschwMSFT, please consider this for approval. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approved. we will take for consideration in 8.0.x
Backport of #95530 to release/8.0-staging
/cc @EgorBo
Customer Impact
When jit needed to zero a struct with gc references it used to emit a call to an internal helper
JIT_MemSet
that didn't have atomicity guarantees and could lead to torn values if GC happened during that memset. This issue was present from .NET core 3.Example:
The PR fixes that issue by inlining memset so we can 100% guarantee atomicity by using correct instructions.
The original issue was found via internal escalation (service crashing 3x per day).
Testing
Local testing + stress/outerloop jobs
Risk
Low. It is not a common case. It happens for a big struct with GC types. Even our SPMI has very few of these cases.